-
Notifications
You must be signed in to change notification settings - Fork 461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safety check for full pruning #5550
Conversation
…or-full-pruning # Conflicts: # src/Nethermind/Nethermind.Blockchain.Test/FullPruning/FullPrunerTests.cs
src/Nethermind/Nethermind.Blockchain/FullPruning/PruningStatus.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # src/Nethermind/Nethermind.Blockchain.Test/FullPruning/FullPrunerTests.cs # src/Nethermind/Nethermind.Blockchain.Test/FullPruning/FullPruningDiskTest.cs # src/Nethermind/Nethermind.Blockchain/FullPruning/FullPruner.cs # src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs
# Conflicts: # src/Nethermind/Nethermind.Init/Steps/InitializeBlockchain.cs
@deffrian @MarekM25 |
|
@@ -158,6 +170,28 @@ private void SetCurrentPruning(IPruningContext pruningContext) | |||
|
|||
private bool CanStartNewPruning() => _fullPruningDb.CanStartPruning; | |||
|
|||
private const double ChainSizeThresholdFactor = 1.3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would make this a long
and make it in %, set it to 130, then when using it first multiply by it, then divide by 100. You keep everything integer and avoid floats.
long? currentChainSize = _chainEstimations.PruningSize; | ||
if (currentChainSize is null) | ||
{ | ||
if (_logger.IsWarn) _logger.Warn("Full Pruning: Chain size estimation is unavailable."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this user problem?
/// <summary> | ||
/// Pruning failed because of low disk space | ||
/// </summary> | ||
NotEnoughDiskSpace, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor - thinking if we shoulnd't keep this enum in some order, so this would go after Disabled
@@ -63,5 +63,8 @@ public interface IPruningConfig : IConfig | |||
"'AlwaysShutdown': shuts Nethermind down once the prune completes, whether it succeeded or failed.", | |||
DefaultValue = "None")] | |||
FullPruningCompletionBehavior FullPruningCompletionBehavior { get; set; } | |||
|
|||
[ConfigItem(Description = "Enables available disk space check.", DefaultValue = "true", HiddenFromDocs = true)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want this to be hidden from docs?
@@ -314,7 +315,10 @@ private Task InitBlockchain() | |||
api.PruningTrigger.Add(pruningTrigger); | |||
} | |||
|
|||
FullPruner pruner = new(fullPruningDb, api.PruningTrigger, pruningConfig, api.BlockTree!, stateReader, api.ProcessExit!, api.LogManager); | |||
IDriveInfo drive = api.FileSystem.GetDriveInfos(fullPruningDb.GetPath(initConfig.BaseDbPath))[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StateDb Path should be used, this can be mapped to different drive.
BlockchainIds.Goerli => new ChainEstimations( | ||
new LinearExtrapolation(8490.MB(), 15.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.Rinkeby => new ChainEstimations( | ||
new LinearExtrapolation(34700.MB(), 20.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.Ropsten => new ChainEstimations( | ||
new LinearExtrapolation(35900.MB(), 25.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.Mainnet => new ChainEstimations( | ||
new LinearExtrapolation(90000.MB(), 70.MB(), new DateTime(2022, 04, 7)), | ||
new LinearExtrapolation(163.GB(), new DateTime(2023, 4, 11, 19, 49, 0), | ||
177653453177, new DateTime(2023, 04, 29, 00, 50, 0))), | ||
BlockchainIds.Gnosis => new ChainEstimations( | ||
new LinearExtrapolation(18000.MB(), 48.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.EnergyWeb => new ChainEstimations( | ||
new LinearExtrapolation(15300.MB(), 15.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.Volta => new ChainEstimations( | ||
new LinearExtrapolation(17500.MB(), 10.MB(), new DateTime(2021, 11, 7))), | ||
BlockchainIds.PoaCore => new ChainEstimations( | ||
new LinearExtrapolation(13900.MB(), 4.MB(), new DateTime(2021, 12, 7))), | ||
BlockchainIds.Sepolia => new ChainEstimations(null, | ||
new LinearExtrapolation(2457.MB(), new DateTime(2023, 04, 9, 21, 11, 0), | ||
3699505976, new DateTime(2023, 04, 28, 20, 18, 0))), | ||
_ => UnknownChain.Instance | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamilchodola can you update chain sizes post newest tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have problems with chain sizes, they are very unstable. I'm working on it right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should do that a bit in similar way as we had with Pivots.
Tests will store data about chain size and then action will fetch them and apply in code.
Would be worth to read it from json file rather than hardcoding
This could be checked and written in healthchecks |
@kamilchodola I will create new pr with progress logger and any other logging you want |
# Conflicts: # src/Nethermind/Nethermind.Blockchain.Test/FullPruning/FullPrunerTests.cs
This reverts commit ca09576.
This reverts commit ca09576
Fixes Closes Resolves #
#5495
Adds available memory check before execution full pruning
Size estimations ready only for mainnet and sepolia. Will add other networks in few weeks.
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?